Update RTCPTWCCFeedback.cs#1640
Conversation
Fix parsing bugs
sipsorcery
left a comment
sipsorcery
left a comment
There was a problem hiding this comment.
The BitsToStatus method is missing.
Add missing methods
|
Ah yes sorry about that |
| private static readonly Dictionary<TWCCPacketStatusType, ushort> StatusToBits = new Dictionary<TWCCPacketStatusType, ushort> | ||
| { | ||
| { TWCCPacketStatusType.NotReceived, 0 }, | ||
| { TWCCPacketStatusType.ReceivedSmallDelta, 1 }, | ||
| { TWCCPacketStatusType.ReceivedLargeDelta, 2 }, | ||
| { TWCCPacketStatusType.Reserved, 3 } | ||
| }; | ||
|
|
||
| private static readonly Dictionary<ushort, TWCCPacketStatusType> BitsToStatus = StatusToBits.ToDictionary(kvp => kvp.Value, kvp => kvp.Key); |
There was a problem hiding this comment.
Consider using FrozenDictionary<TKey,TValue> for performant unmodifiable dictionaries.
But, for this use case, a switch expression would be the best option.
| int expectedDeltaCount = statusSymbols.Count(s => | ||
| s == TWCCPacketStatusType.ReceivedSmallDelta || | ||
| s == TWCCPacketStatusType.ReceivedLargeDelta); | ||
|
|
There was a problem hiding this comment.
expectedDeltaCount doesn't seem to be used.
This adds an unnecessary enumeration of statusSymbols with delegate invocation.
|
|
||
| private List<TWCCPacketStatusType> ParseStatusChunks(byte[] packet, ref int offset) | ||
| { | ||
| var statusSymbols = new List<TWCCPacketStatusType>(); |
There was a problem hiding this comment.
Initialize the list with the know maximum size to avoid allocations and memory copy on growth.
var statusSymbols = new List<TWCCPacketStatusType>(PacketStatusCount);|
|
||
| private (List<int> deltaValues, int lastOffset) ParseDeltaValues(byte[] packet, int offset, List<TWCCPacketStatusType> statusSymbols) | ||
| { | ||
| var deltaValues = new List<int>(); |
There was a problem hiding this comment.
Initialize the list with the know maximum size to avoid allocations and memory copy on growth.
var deltaValues = new List<int>(statusSymbols.Count);| List<TWCCPacketStatusType> symbols = PacketStatuses.Select(ps => ps.Status).ToList(); | ||
|
|
||
| // Reconstruct packet status chunks. | ||
| List<ushort> chunks = new List<ushort>(); |
There was a problem hiding this comment.
Initialize the list with the know maximum size to avoid allocations and memory copy on growth.
// Worst-case chunk count is all 2-bit vector chunks (7 symbols per chunk).
int chunkCapacity = (symbolCount + 6) / 7;
List<ushort> chunks = new List<ushort>(chunkCapacity);| public byte[] GetBytes() | ||
| { | ||
| // Build a list of TWCCPacketStatusType from PacketStatuses. | ||
| List<TWCCPacketStatusType> symbols = PacketStatuses.Select(ps => ps.Status).ToList(); |
There was a problem hiding this comment.
Using LINQ erases the optimizations that List<T> have. Also grows a list that is known to have a maximum and uses delegate invocation for that.
int symbolCount = PacketStatuses.Count;
var symbols = new List<TWCCPacketStatusType>(symbolCount);
int deltaBytesCapacity = 0;
foreach (var ps in PacketStatuses)
{
symbols.Add(ps.Status);
if (ps.Status == TWCCPacketStatusType.ReceivedSmallDelta)
{
deltaBytesCapacity += 1;
}
else if (ps.Status == TWCCPacketStatusType.ReceivedLargeDelta)
{
deltaBytesCapacity += 2;
}
}| if (!BitsToStatus.TryGetValue(statusBits, out TWCCPacketStatusType symbol)) | ||
| { | ||
| throw new ArgumentException($"Invalid status bits in run length chunk: {statusBits}"); | ||
| } |
There was a problem hiding this comment.
Consider a switch expression.
Doesn't require dictionary allocation and lookup,
| if (!BitsToStatus.TryGetValue(statusBits, out TWCCPacketStatusType symbol)) | |
| { | |
| throw new ArgumentException($"Invalid status bits in run length chunk: {statusBits}"); | |
| } | |
| TWCCPacketStatusType symbol = statusBits switch | |
| { | |
| 0 => TWCCPacketStatusType.NotReceived, | |
| 1 => TWCCPacketStatusType.ReceivedSmallDelta, | |
| 2 => TWCCPacketStatusType.ReceivedLargeDelta, | |
| 3 => TWCCPacketStatusType.Reserved, | |
| _ => throw new ArgumentException($"Invalid status bits in run length chunk: {statusBits}") | |
| }; |
3 participants
Fix parsing bugs